I'm not particularly sure what to test, any suggestions on test cases to add? Maybe some common operations like squash, rebase, git fetch and push?

I'm also thinking that templates are a good idea, maybe something like ModifiedChange nodes and ModifiedBranch nodes if that makes sense? Although the commit summary is a good starting point, seeing things like hidden repeatedly on removed commits isn't particularly useful. I'm not sure if the branch status is useful as well since, it will also be shown in the modified branches section.

I'm not particularly sure what to test, any suggestions on test cases to add? Maybe some common operations like squash, rebase, git fetch and push?

I think you need to exercise the code for each option that the command handles.
If you expect edge cases for different operations, test that. I expect undo to work the same as other commands, but a test should prove it. Maybe abandon or branch create would be good to test that not just the commits but the meta data of the operation is shown or diffed correctly.

@bnjmnt4n bnjmnt4n marked this pull request as ready for review 2 weeks ago

Sorry, I missed this event. GitHub doesn't send an email for it. I'll try to remember to review this later today.

I think this really should have some tests. I'll review the code now.

I'll try to add some testcases when I have the time. Here's the list of operations I think we should test (feel free to suggest more):

• new
• abandon
• squash
• rebase
• branch create
• git fetch
• git push
• op undo
• op restore

I'm not sure how to structure it though: I guess I should add a single test beginning from an empty repo for each of these operations?

Thanks for taking a look @yuja. I'll try to work on adding some tests to make it easier to see what the expected output is like, perhaps further reviews can be put on hold if you'd like.

I'm also thinking that templates are a good idea, maybe something like ModifiedChange nodes and ModifiedBranch nodes if that makes sense? Although the commit summary is a good starting point, seeing things like hidden repeatedly on removed commits isn't particularly useful. I'm not sure if the branch status is useful as well since, it will also be shown in the modified branches section.

I'd also like to see if there's any desire to change the output displayed.

I'll try to add some testcases when I have the time. Here's the list of operations I think we should test (feel free to suggest more):

• new
• abandon
• squash
• rebase
• branch create
• git fetch
• git push

As @joyously said, it's probably best to instead focus on what the code does and the different cases it handles. For example, there's some logic for displaying how refs moved, so there should be some tests demonstrating that, and ideally some including conflicted refs. It doesn't matter which commands you use to create the refs, so use whatever is easiest for that (for creating conflicted branches, it's probably easiest to use jj branch set --at-op=...).

• op undo
• op restore

I don't think these two need to be tested. They don't do much you can't test by making the same changes explicitly.

I'm not sure how to structure it though: I guess I should add a single test beginning from an empty repo for each of these operations?

SGTM.

I've updated this PR with tests. Would appreciate input on whether the output should change in any way, as well as any changes to the tests.

@yuja would appreciate if you could take another look. I've updated to avoid creating a transaction for jj op show, and used the new commit_summary_template helper as well.